Skip to content

feat(finding): multiple CWEs per finding#15143

Merged
Maffooch merged 10 commits into
devfrom
feat/cwe-vuln-id-consolidation
Jul 14, 2026
Merged

feat(finding): multiple CWEs per finding#15143
Maffooch merged 10 commits into
devfrom
feat/cwe-vuln-id-consolidation

Conversation

@valentijnscholten

@valentijnscholten valentijnscholten commented Jul 2, 2026

Copy link
Copy Markdown
Member

🔗 PR stack — CWE / vulnerability-ID consolidation (OSS)

Merge bottom-up:

  • #15145 — autodetected vulnerability-ID type + uniqueness constraint
    • #15143 — multiple CWEs per finding
      • #15154 — docs: Pro set-based dedup hash-code fields
      • #15155 — pluggable false-positive-history candidate filter

👉 This PR: #15143


Stacked on top of: #15145

Description

A finding could previously store only one CWE — the integer Finding.cwe field. This PR lets a finding carry multiple CWEs, using the same approach we already use for vulnerability ids (CVE, GHSA, …):

  • The primary CWE stays on Finding.cwe (unchanged). Legacy behaviour, hash codes and existing deduplication continue to use it exactly as before.
  • Additional CWEs are stored in a new Finding_CWE relationship (unique per (finding, cwe)), exactly like Vulnerability_Id rows hang off a finding for vulnerability ids.
  • The finding exposes the merged list via finding.cwes (primary first, then additional, deduplicated) — mirroring the existing finding.vulnerability_ids property.

CWEs can now be added in three places, matching how vulnerability ids already flow through the system:

  • UI — the finding add/edit forms and the finding detail page accept and display multiple CWEs.
  • API — findings expose a cwes list field that reads and writes the additional CWEs, mirroring the existing vulnerability_ids field.
  • Parsers — parsers can populate finding.unsaved_cwes; on import the primary CWE plus any additional CWEs the parser supplies are persisted. Tier-2 parsers that surface more than one CWE per finding now emit all of them.

CWEs are stored canonically as CWE-<n> strings. A CWE is a weakness class, not a vulnerability instance identifier, so it is modelled separately from vulnerability ids and does not participate in hash_code or the cve field. Because of that separation, existing hash codes and deduplication are unaffected by this change.

Stacked on top of #15145. This PR is stacked on the vulnerability-id-type change (#15145). Its base is dev, so until #15145 merges the diff/commits below also include #15145's changes — review and merge #15145 first, then this PR. The CWE-specific work is the top commit; its migrations 0279_finding_cwe / 0280_backfill_finding_cwe chain after #15145's 02760278. After #15145 merges to dev, this PR's diff will reduce to the CWE-only changes.

Migrations (this PR adds the last two; the first three come from #15145)

  • 0279_finding_cwe — creates the Finding_CWE table (unique per (finding, cwe)).
  • 0280_backfill_finding_cwe — seeds Finding_CWE rows from the legacy Finding.cwe values.

To backfill CWEs for existing findings after upgrading, run the idempotent command:

manage.py migrate_cwe

Test results

  • New unittests/test_finding_cwe.py covers the CWE string helpers, the Finding_CWE model, the finding.cwes property, copy-on-clone, and the cwes API field.
  • Parser tests updated/added across ~30 parsers (with new multi-CWE scan fixtures) to assert that multiple CWEs are parsed and persisted per finding.
  • Performance baselines (test_importers_performance.py, test_tag_inheritance_perf.py) updated for the added Finding_CWE store/reconcile queries.

Documentation

  • docs/content/releases/os_upgrading/3.2.md describes both stacked changes, the migrations, and the migrate_cwe backfill command.
image image image

@github-actions github-actions Bot added New Migration Adding a new migration file. Take care when merging. docs unittests ui parser labels Jul 2, 2026
@valentijnscholten valentijnscholten added this to the 3.2.0 milestone Jul 2, 2026
@valentijnscholten
valentijnscholten force-pushed the feat/cwe-vuln-id-consolidation branch from e0216e4 to e984c77 Compare July 2, 2026 20:37
@valentijnscholten
valentijnscholten marked this pull request as ready for review July 2, 2026 21:06

@mtesauro mtesauro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved

@github-actions github-actions Bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR conflicts-detected labels Jul 5, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Maffooch
Maffooch requested a review from blakeaowens as a code owner July 14, 2026 00:24
@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from 354058e to 518df6c Compare July 14, 2026 01:07
@github-actions

Copy link
Copy Markdown
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from da8c84a to 2f3a611 Compare July 14, 2026 02:13
valentijnscholten and others added 8 commits July 13, 2026 21:09
Stacked on top of the vulnerability_id-type change (feat/vulnerability-id-type).

Adds a Finding_CWE relationship so a finding can carry multiple CWEs, mirroring
vulnerability ids: the primary CWE stays on Finding.cwe; additional CWEs live in
the relationship and are exposed via finding.cwes. Wired through the UI, the API
(cwes field), and parsers (finding.unsaved_cwes). CWE is a weakness class, kept
out of hash_code and the cve field, so existing hash codes/dedup are unaffected.

Migrations 0279_finding_cwe (create table) and 0280_backfill_finding_cwe (seed
from legacy Finding.cwe), chained after the vulnerability_id migrations (0278).
Backfill existing findings with: manage.py migrate_cwe
The generic JSON and CSV importers emitted only a single `cwe` per finding,
so a finding could not be imported with a CWE *set*. Mirror the existing
`vulnerability_ids` handling to accept multiple CWEs:

- JSON: accept an optional `cwes` list (popped like `vulnerability_ids`,
  since it is not a Finding field). The first entry becomes the primary
  `finding.cwe` when `cwe` is not already given; the full set is kept on
  `finding.unsaved_cwes`. Mixed int / "CWE-<n>" forms are supported.
- CSV: accept a delimited `CweIds` column alongside the existing single
  `CweId`, with the same primary-plus-set behavior; merge CWE sets across
  internally-deduplicated rows like vulnerability ids.

The import pipeline persists the set via `Finding_CWE`; normalization and
deduplication happen in `finding_cwe_labels()`. The legacy single `cwe`
path is unchanged.

Adds a fabricated multi-CWE fixture and parser tests (JSON + CSV).
Add 'cwes' to HASHCODE_ALLOWED_FIELDS and hash it via a new get_cwes(), which
mirrors get_vulnerability_ids. Because the primary cwe is a scalar that is always
set (so Finding.cwes is non-empty even before the extra Finding_CWE rows are
written during import), get_cwes prefers unsaved_cwes whenever present, so the
pre-save and post-save import hashes agree.

Tests: get_cwes prefers unsaved_cwes, is stable across save, empty with no cwe,
and cwes participates in compute_hash_code.
Mirror the existing cve/vulnerability_ids deprecation note: the single primary
cwe field is merged into the cwes set for backward compatibility and can be
removed once no longer needed.
… cached property

get_cwes() used the cwes @cached_property for the saved path; if finding.cwes was
accessed before the Finding_CWE rows were written (serializer/signal/log), it cached
an empty/partial set and the hash then used that stale value -> nondeterministic
hash_code (flaky dedup under parallel test runs). Mirror get_vulnerability_ids: read
the finding_cwe_set reverse relation directly (uncached, prefetch-honoring), falling
back to the unsaved values before save. save_cwes stores the primary cwe as a row too,
so finding_cwe_set carries the full set.
Move cwe_number, cwe_label, parse_cwes, finding_cwe_labels out of
finding/vulnerability_id.py into a dedicated finding/cwe.py (CWEs are a weakness
class, distinct from vulnerability identifiers). resolve_vulnerability_id_type
stays in vulnerability_id.py. All importers updated.
…mn width

Two data-safety fixes for the multi-CWE relation:

- FindingSerializer.update() called save_cwes() unconditionally, so any
  PATCH/PUT that omitted `cwes` deleted a finding's extra Finding_CWE rows
  (leaving only the primary). Guard the call on `parsed_cwes is not None`,
  mirroring the vulnerability-id path.

- cwe_number() had no upper bound, but Finding_CWE.cwe stores "CWE-<n>" in a
  varchar(11). A number > 9,999,999 passed validation then raised DataError
  at insert (API/form 500, and would abort the 0280 backfill's bulk_create,
  since ignore_conflicts does not swallow DataError). Reject it as invalid
  input (None) so it becomes a clean 400/skip.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restacked onto the renumbered vulnerability-id chain; the CWE migrations move
0279/0280 -> 0283/0284 and re-parent onto 0282_unique_finding_vulnerability_id
so the branch has a single linear migration leaf again.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ep (F10)

- Update the migration list to the renumbered chain (0280-0284).
- Remove the contradiction: 0284_backfill_finding_cwe already backfills every
  existing finding automatically, so migrate_cwe is NOT a required upgrade
  step (it would repeat that work). Reframe migrate_cwe as an optional,
  idempotent re-scan for later use (e.g. after a parser CWE change).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch
Maffooch force-pushed the feat/cwe-vuln-id-consolidation branch from 2f3a611 to 0b291ea Compare July 14, 2026 03:15
The pghistory_no_async_with_product_grading counts were placeholders from the
pre-merge branch base. Update import1/reimport1 to the actual counts on the
merged dev (from CI): the multi-CWE work adds ~2 queries per import phase and
the async-task counts follow dev's current values.

  Small:          import1 182q/5t, reimport1 141q/4t
  SmallLocations: import1 194q/5t, reimport1 155q/4t

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Maffooch
Maffooch merged commit 3edfceb into dev Jul 14, 2026
150 checks passed
@Maffooch
Maffooch deleted the feat/cwe-vuln-id-consolidation branch July 14, 2026 03:54
Maffooch pushed a commit that referenced this pull request Jul 23, 2026
…pplied (#15305)

PR #15143 added multiple-CWE support (cwes object list, writable on
create and update). When a request supplied BOTH the scalar cwe and a
cwes list, cwes[0] unconditionally overwrote the explicit scalar - the
more specific input lost.

Precedence now mirrors the vulnerability_ids->cve pattern: an explicit
scalar cwe in the request stays the primary and every cwes entry is
treated as an extra row (save_cwes dedupes the overlap); without a
scalar, cwes[0] becomes the primary and is mirrored into the scalar.
Detection uses initial_data presence - the established pattern in this
serializer (FindingTemplateSerializer).

Behavior change only for the unreleased cwes field when both inputs
appear in one request; no released consumer can depend on the old
behavior. Wire shape unchanged.

+8 API tests: precedence on create/update, cwes-only primary promotion,
replace, omission-untouched, explicit-empty semantics, read-back shape.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs New Migration Adding a new migration file. Take care when merging. parser settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR ui unittests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants